Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unskips visualize integration tests #142917

Merged

Conversation

TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Oct 6, 2022

Fix #89958
Fix #88639

Summary:
Unskips tests as these pass locally as-is without any changes to see if post-8.0 resolved the flakiness.

Note: 1/50 flaky test runs failed. Still WIP for now.
Update: Attempt2
Flaky test run succeeded 50/50 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1428

@TinaHeiligers TinaHeiligers added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Saved Object Tagging Saved Objects Tagging feature backport:all-open Backport to all branches that could still receive a release v8.6.0 labels Oct 6, 2022
@TinaHeiligers TinaHeiligers requested a review from a team as a code owner October 6, 2022 20:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@TinaHeiligers TinaHeiligers added the release_note:skip Skip the PR/issue when compiling release notes label Oct 6, 2022
@TinaHeiligers TinaHeiligers marked this pull request as draft October 6, 2022 23:17
@TinaHeiligers TinaHeiligers force-pushed the kbn-89958-visualize-tagging-fail branch from 6a4bf73 to 6b8183e Compare October 13, 2022 22:53
@@ -467,7 +467,7 @@ export class CommonPageObject extends FtrService {
async waitForSaveModalToClose() {
this.log.debug('Waiting for save modal to close');
await this.retry.try(async () => {
if (await this.testSubjects.exists('savedObjectSaveModal')) {
if (await this.testSubjects.exists('savedObjectSaveModal', { timeout: 5000 })) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

increase timeout

@TinaHeiligers TinaHeiligers marked this pull request as ready for review October 14, 2022 00:21
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor Author

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review

@@ -51,6 +51,16 @@ export class ListingTableService extends FtrService {
return visualizationNames;
}

private async getAllSelectableItemsNamesOnCurrentPage(): Promise<string[]> {
const visualizationNames = [];
const links = await this.find.allByCssSelector('.euiTableRow-isSelectable .euiLink');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.euiTableRow .euiLink matches on an empty table. I chose to add a new method rather than risk affecting other tests that implement the method.

* Navigates through all pages on Landing page and returns array of items names that are selectable
* Added for visualize_integration saved object tagging tests
*/
public async getAllSelectableItemsNames(): Promise<string[]> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implements selecting "selectable" items


},
{
"id": "myextratag",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tag for adding to a visualization that's been edited

// Failing: See https://github.com/elastic/kibana/issues/89958
describe.skip('visualize integration', () => {
await testSubjects.click('confirmSaveSavedObjectButton');
await retry.waitForWithTimeout('Save modal to disappear', 5000, () =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Increased timeout waiting for the modal to clear

before(async () => {
// clean up any left-over visualizations and tags from tests that didn't clean up after themselves
await kibanaServer.savedObjects.clean({ types: ['tag', 'visualization'] });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that if previous tests hadn't cleaned up after themselves, we get a mismatch on the visualizations remaining.

@@ -77,48 +115,46 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
it('allows to manually type tag filter query', async () => {
await listingTable.searchForItemWithName('tag:(tag-1)', { escape: false });
await listingTable.expectItemsCount('visualize', 2);
const itemNames = await listingTable.getAllItemsNames();
const itemNames = await listingTable.getAllSelectableItemsNames();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implements selecting all visualizations by 'euiTableRow-isSelectable`


await testSubjects.click('confirmSaveSavedObjectButton');
await PageObjects.common.waitForSaveModalToClose();
await createSimpleMarkdownVis({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uses helper method


await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();

await selectFilterTags('tag-1');
const itemNames = await listingTable.getAllItemsNames();
await selectFilterTags('myextratag');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot guarantee the order in which the tests in this suite will run or if any are skipped again in the future, so we use a different tag to add to an existing visualization.

await PageObjects.visualize.gotoVisualizationLandingPage();
await listingTable.waitUntilTableIsLoaded();
await PageObjects.visualize.deleteAllVisualizations();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding all links by css-matchers takes time and causes timeouts. We clean up all the other visualizations to increase rendering speed.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💚 Build #78871 succeeded 6a4bf73cafa59f17ea0aaee45fc636a494512c41

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!!! @TinaHeiligers thank you for fixing the tests!! It LGTM!

await PageObjects.visualize.ensureSavePanelOpen();
await selectSavedObjectTags('tag-2');

await selectSavedObjectTags(...['myextratag']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we need the spread?

Suggested change
await selectSavedObjectTags(...['myextratag']);
await selectSavedObjectTags('myextratag');

@TinaHeiligers TinaHeiligers merged commit 0b10d6c into elastic:main Oct 14, 2022
@TinaHeiligers TinaHeiligers deleted the kbn-89958-visualize-tagging-fail branch October 14, 2022 13:51
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 14, 2022
Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 0b10d6c)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
7.17 Backport failed because of merge conflicts
8.5

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 142917

Questions ?

Please refer to the Backport tool documentation

@TinaHeiligers TinaHeiligers added backport:skip This commit does not require backporting and removed backport:all-open Backport to all branches that could still receive a release labels Oct 14, 2022
kibanamachine added a commit that referenced this pull request Oct 14, 2022
Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 0b10d6c)

Co-authored-by: Christiane (Tina) Heiligers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment